UIEXT-3109: Migrate node #4

tom-mi · 2025-12-16T09:37:44Z

UIEXT-3109 (WebUI-Migration Excel Table Writer)

Copilot

Pull request overview

This PR migrates the Excel Table Writer node to use the modern node parameter framework, replacing the legacy dialog implementation with declarative parameter definitions. The migration introduces type-safe parameter handling and modern UI capabilities while maintaining backward compatibility with existing workflows.

Key changes:

Replaced legacy NodeDialogPane with declarative NodeParameters class
Added @Label annotations to enums for improved UI presentation
Implemented comprehensive unit tests for the new parameter structure

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
PaperSize.java	Added `@Label` annotations to enum values for UI display names
Orientation.java	Added `@Label` annotations to enum values for UI display names
ExcelTableWriterNodeParameters.java	New file containing declarative node parameter definitions replacing legacy dialog
ExcelTableWriterNodeFactory.xml	Removed legacy XML-based node description
ExcelTableWriterNodeFactory.java	Updated to implement new dialog and description generation interfaces
ExcelTableWriterConfig.java	Changed field visibility from private to package-private for parameter access
ExcelEncryptionSettings.java	New reusable parameter widget for Excel encryption configuration
ExcelTableWriterNodeParametersTest.java	New test file with snapshot tests and dynamic update validation
Test snapshot files	Generated test snapshots for parameter structure validation
MANIFEST.MF	Added testing dependency for UI parameter testing

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

...i3/src/org/knime/ext/poi3/node/io/filehandling/excel/writer/ExcelTableWriterNodeFactory.java

...nime.ext.poi3/src/org/knime/ext/poi3/node/io/filehandling/excel/ExcelEncryptionSettings.java

Copilot

Pull request overview

Copilot reviewed 14 out of 15 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

UIEXT-3109 (WebUI-Migration Excel Table Writer)

Copilot

Pull request overview

Copilot reviewed 14 out of 15 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

marcbux · 2026-01-06T12:02:01Z

...src/org/knime/ext/poi3/node/io/filehandling/excel/writer/ExcelTableWriterNodeParameters.java

+                + "isn't set to append, each sheet name must be unique. The node appends the tables in the order they "
+                + "are connected.")
+        @SuggestionsProvider(SheetNamesChoicesProvider.class)
+        String m_sheetName = "Sheet1";


Can we change this such that. just like in the old dialog, the default is something unique? Right now we get this behavior, which is not ideal:

I restored the old behaviour of using default_${i+1} as default sheet name.

(Note that this caused two more PRs for knime-base & knime-core-ui)

marcbux · 2026-01-06T12:03:59Z

...src/org/knime/ext/poi3/node/io/filehandling/excel/writer/ExcelTableWriterNodeParameters.java

+    @ValueSwitchWidget
+    @Persist(configKey = ExcelTableWriterConfig.CFG_SHEET_EXISTS)
+    @Layout(SheetsSection.class)
+    @Effect(predicate = OutputFileOverwritePolicyIsAppend.class, type = Effect.EffectType.SHOW)


As mentioned on Slack, while I am OK with keeping the behavior of this value switch as it was in the old dialog, I prefer your suggested quick fix. Since the If-sheet-exists option does have an effect on the output when the If-exists option is set to Overwrite, it should be shown imho.

Please have another look if you are happy with the current implementation.
I'm not 100% sure if the way I coupled the checkbox & the choices widget is the best solution or if there is a more elegant way.

marcbux · 2026-01-06T12:07:07Z

...src/org/knime/ext/poi3/node/io/filehandling/excel/writer/ExcelTableWriterNodeParameters.java

+        elementLayout = ElementLayout.HORIZONTAL_SINGLE_LINE)
+    @ValueProvider(SheetNamesValueProvider.class)
+    @Persistor(SheetNamesPersistor.class)
+    @Migrate


What does the @Migrate annotation do here?

It makes the behavior explicit (no defaults are loaded), as the @LoadDefaultsForAbsentFields in combination with a custom @Persistor, therefore the syntax check syntax check will warn if there is no explicit @Migrate or @Migrator annotation. In this case, it will turn off the default (as far as I understood it).

marcbux · 2026-01-06T12:09:08Z

...org/knime/ext/poi3/node/io/filehandling/excel/writer/ExcelTableWriterNodeParametersTest.java

+        var result = simulator.simulateValueChange("outputFile", "file");
+
+        // then
+        assertThat(


this test fails on Jenkins: https://jenkins.knime.com/blue/organizations/jenkins/knime-excel/detail/PR-4/10/tests

Fixed by adding a dependency to org.knime.features.filehandling.core in pom.xml (see discussion in Slack)

marcbux · 2026-01-06T12:19:36Z

...i3/src/org/knime/ext/poi3/node/io/filehandling/excel/writer/ExcelTableWriterNodeFactory.java

+                    appendend to the last sheet in the name sequence. The original sheet name does not have to be changed.
+                    </p> <p>This node does not support writing files in the '.xlsm' format, yet appending is supported.</p>
+                    <p> <i>This node can access a variety of different</i> <a
+                    href="https://docs.knime.com/2021-06/analytics_platform_file_handling_guide/index.html#analytics-platform-file-systems"><i>file


This URL is outdated (2021-06).

Copilot

Pull request overview

Copilot reviewed 14 out of 15 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

org.knime.ext.poi3/src/org/knime/ext/poi3/node/io/filehandling/excel/writer/ExcelTableWriterNodeParameters.java:1

The suppression of SonarQube rule S103 (line length) is applied to a multi-line string literal (FULL_DESCRIPTION). Consider refactoring the description string to use proper line breaks and concatenation instead of suppressing the warning, which would improve code readability.

/*

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 14 out of 15 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

org.knime.ext.poi3/src/org/knime/ext/poi3/node/io/filehandling/excel/writer/ExcelTableWriterNodeParameters.java:1

Corrected spelling of 'appendend' to 'appended'.

/*

org.knime.ext.poi3/src/org/knime/ext/poi3/node/io/filehandling/excel/writer/ExcelTableWriterNodeParameters.java:1

Add a comment explaining why this SonarQube rule suppression is necessary. The rule S103 is about line length, so the comment should clarify why long lines are acceptable in this specific FULL_DESCRIPTION string constant.

/*

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

UIEXT-3109 (WebUI-Migration Excel Table Writer)

* Needed to run tests on jenkins UIEXT-3109 (WebUI-Migration Excel Table Writer)

UIEXT-3109 (WebUI-Migration Excel Table Writer)

Copilot

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

org.knime.ext.poi3/src/org/knime/ext/poi3/node/io/filehandling/excel/writer/ExcelTableWriterNodeParameters.java:1

The SonarQube warning suppression for S103 (line length) should be replaced with proper line breaking in the string literal. Modern Java text blocks support line breaks naturally and don't require this suppression.

/*

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

...i3/src/org/knime/ext/poi3/node/io/filehandling/excel/writer/ExcelTableWriterNodeFactory.java

sonarqubecloud · 2026-01-08T15:27:27Z

Quality Gate failed

Failed conditions
84.8% Coverage on New Code (required ≥ 85%)

See analysis details on SonarQube Cloud

tom-mi requested a review from a team as a code owner December 16, 2025 09:37

Copilot AI review requested due to automatic review settings December 16, 2025 09:37

Copilot AI reviewed Dec 16, 2025

View reviewed changes

tom-mi force-pushed the enh/UIEXT-3109-webui-migration-excel-table-writer branch from 7ad3428 to 744b24e Compare December 16, 2025 09:47

Copilot AI review requested due to automatic review settings December 16, 2025 12:54

Copilot AI reviewed Dec 16, 2025

View reviewed changes

...i3/src/org/knime/ext/poi3/node/io/filehandling/excel/writer/ExcelTableWriterNodeFactory.java Show resolved Hide resolved

...nime.ext.poi3/src/org/knime/ext/poi3/node/io/filehandling/excel/ExcelEncryptionSettings.java Show resolved Hide resolved

Copilot AI review requested due to automatic review settings December 16, 2025 14:30

Copilot AI reviewed Dec 16, 2025

View reviewed changes

tom-mi force-pushed the enh/UIEXT-3109-webui-migration-excel-table-writer branch from fa7efc6 to 3f7a02e Compare December 17, 2025 08:28

UIEXT-3109: Migrate node

49464ad

UIEXT-3109 (WebUI-Migration Excel Table Writer)

Copilot AI review requested due to automatic review settings December 18, 2025 12:53

tom-mi force-pushed the enh/UIEXT-3109-webui-migration-excel-table-writer branch from 3f7a02e to 49464ad Compare December 18, 2025 12:53

Copilot AI reviewed Dec 18, 2025

View reviewed changes

marcbux requested changes Jan 6, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings January 8, 2026 08:12

Copilot AI reviewed Jan 8, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings January 8, 2026 09:30

Copilot AI reviewed Jan 8, 2026

View reviewed changes

tom-mi force-pushed the enh/UIEXT-3109-webui-migration-excel-table-writer branch from 8939b62 to ed50db0 Compare January 8, 2026 14:35

tom-mi added 4 commits January 8, 2026 16:13

UIEXT-3109: Generate unique default sheet name

7972dae

UIEXT-3109 (WebUI-Migration Excel Table Writer)

UIEXT-3109: Add support for local filesystems in node tests

6eba6f0

* Needed to run tests on jenkins UIEXT-3109 (WebUI-Migration Excel Table Writer)

UIEXT-3109: Add logging for exceptions during sheet name retrieval

be667be

UIEXT-3109 (WebUI-Migration Excel Table Writer)

UIEXT-3109: Fix outdated documentation link

b4cdce9

UIEXT-3109 (WebUI-Migration Excel Table Writer)

Copilot AI review requested due to automatic review settings January 8, 2026 15:14

tom-mi force-pushed the enh/UIEXT-3109-webui-migration-excel-table-writer branch from ed50db0 to b4cdce9 Compare January 8, 2026 15:14

Copilot AI reviewed Jan 8, 2026

View reviewed changes

...i3/src/org/knime/ext/poi3/node/io/filehandling/excel/writer/ExcelTableWriterNodeFactory.java Show resolved Hide resolved

UIEXT-3109: Migrate node #4

Are you sure you want to change the base?

UIEXT-3109: Migrate node #4

Uh oh!

Conversation

tom-mi commented Dec 16, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

sonarqubecloud bot commented Jan 8, 2026

Quality Gate failed

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants